-
Notifications
You must be signed in to change notification settings - Fork 593
feat: enable new architecture #12825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
6cb25b0 to
841f693
Compare
841f693 to
f037931
Compare
|
I am opening the PR to make sure all CI stuff keeps on running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
Enables React Native new architecture experiment by setting newArchEnabled=true on both iOS and Android platforms and converting native modules to C++ (.mm).
- Enabled new architecture in
android/gradle.properties,ios/Podfile, andios/Podfile.properties.json - Converted
ARAppDelegateHelper.mandARPHPhotoPickerModule.mto.mm(Objective-C++) for new arch compilation compatibility - Updated
react-native-context-menu-viewandreact-native-launch-argumentsto fork branches with new architecture fixes - Added
startTransitionprop toFlagProviderto handle React 18 transitions - Bumped node cache version to v26
Issues found:
- Logic error in photo picker: checking
results.countinstead ofimages.countafter image loading could cause incorrect error handling
Additional Comments (1)
-
ios/Artsy/Emission/EigenCommunications/ARPHPhotoPickerModule.mm, line 96 (link)logic: checking
results.count == 0here is incorrect - should checkimages.count == 0since images may fail to load even when results exist
9 files reviewed, 2 comments
|
|
||
| #import <AuthenticationServices/AuthenticationServices.h> | ||
| #import <SafariServices/SafariServices.h> | ||
| #import <FBSDKCoreKit/FBSDKCoreKit-swift.h> | ||
|
|
||
|
|
||
| #import <Firebase.h> | ||
| #import "BrazeReactBridge.h" | ||
| #import "BrazeReactUtils.h" | ||
| #import <BrazeUI/BrazeUI-Swift.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: added imports for AuthenticationServices and SafariServices when converting to C++, but these frameworks don't appear to be used - consider removing unused imports
f037931 to
ab97692
Compare
zeegoAuthor: Fernando Rojo Description: Logical UI primitives, made for screens. Homepage: https://github.com/nandorojo/zeeg#readme
|
| Created | over 3 years ago |
| Last Updated | 4 months ago |
| License | MIT |
| Maintainers | 1 |
| Releases | 167 |
| Keywords | react-native, ios, utilities, utility, fabric, paper and JSI |
react-native-ios-context-menu
Author: Unknown
Description: A react-native component to use context menu's (UIMenu) on iOS 13/14+
Homepage: https://github.com/dominicstop/react-native-ios-context-menu#readme
| Created | about 5 years ago |
| Last Updated | 4 months ago |
| License | MIT |
| Maintainers | 1 |
| Releases | 117 |
| Direct Dependencies | @dominicstop/ts-event-emitter |
| Keywords | react-native, ios, react-native-ios-context-menu and ReactNativeIosContextMenu |
@react-native-menu/menu
Author: Jesse Katsumata
Description: UIMenu component for react-native
Homepage: https://github.com/react-native-menu/menu#readme
| Created | about 5 years ago |
| Last Updated | 4 months ago |
| License | MIT |
| Maintainers | 1 |
| Releases | 41 |
| Keywords | react-native, ios and android |
New dependencies added: @react-native-menu/menu, react-native-ios-context-menu, react-native-ios-utilities and zeego.
01c525d to
17f792a
Compare
17f792a to
32d96d6
Compare
src/app/App.tsx
Outdated
|
|
||
| require("./system/ignoreLogs") | ||
|
|
||
| enableFreeze(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to look into again
| }), | ||
| }, | ||
| ], | ||
| left: withTiming(followButtonTranslateX.value, { duration: 200, easing: Easing.sin }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <Swiper | ||
| cards={artworks} | ||
| HeaderComponent={() => <InfiniteDiscoveryHeader topArtwork={topArtwork} />} | ||
| cards={[artworks[0]]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert this
32d96d6 to
2a60107
Compare
2611752 to
918d89a
Compare
🎉 Beta Versions Generated (commit:
|
🎉 Beta Versions Generated (commit:
|
🎉 Beta Versions Generated (commit:
|
This reverts commit 1eb5eff.
Description
This PR is the last of a series of PRs aiming to enable the new architecture in Eigen.
*Things to keep in mind
columnIndexcalculation because the API was dropped fromFlashlist(will be discussed with data team)How to review this PR
We tried as much as possible to approach this commit by commit. That's how I suggest reviewing this PR if you want to know what this journey was like. However, keep in mind that as you progress further in the commits, we change the approach we take accordingly, and sometimes we revert some changes.
QA boards
QA session 1: https://www.notion.so/artsy/2025-10-31-Mobile-App-QA-New-architecture-29dcab0764a0809d831ad7a43c2a6c46
QA session 2: https://www.notion.so/artsy/2025-11-21-Mobile-App-QA-version-8-89-0-iOS-build-2025-11-21-10-Android-build-8-89-0-2025112110--2b2cab0764a08010af36d6b48805aa24
QA session 3: https://www.notion.so/artsy/2025-12-10-Mobile-App-QA-version-8-90-0-iOS-build-2025-12-10-Y-Z-Android-build-XYZ-New-Architectu-2c5cab0764a08025a6e2e0cd25dba760
In Progress
Measuring performance impact
PR Checklist
To the reviewers 👀
Changelog updates
Changelog updates
Cross-platform user-facing changes
iOS user-facing changes
Android user-facing changes
Dev changes
Need help with something? Have a look at our docs, or get in touch with us.